Skip to content

feat(securitypolicy): add MergeType support for policy merging#7918

Open
rajatvig wants to merge 28 commits intoenvoyproxy:mainfrom
rajatvig:feat/security-policy-merge
Open

feat(securitypolicy): add MergeType support for policy merging#7918
rajatvig wants to merge 28 commits intoenvoyproxy:mainfrom
rajatvig:feat/security-policy-merge

Conversation

@rajatvig
Copy link
Contributor

Add MergeType field to SecurityPolicy to enable policy merging similar
to BackendTrafficPolicy. This allows route-level policies to merge with
parent Gateway/Listener policies rather than completely overriding them.

Fixes #6734

@netlify
Copy link

netlify bot commented Jan 12, 2026

Deploy Preview for cerulean-figolla-1f9435 ready!

Name Link
🔨 Latest commit 215388d
🔍 Latest deploy log https://app.netlify.com/projects/cerulean-figolla-1f9435/deploys/699317c9dbfabd00077fe8cf
😎 Deploy Preview https://deploy-preview-7918--cerulean-figolla-1f9435.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

  Add MergeType field to SecurityPolicy to enable policy merging similar
  to BackendTrafficPolicy. This allows route-level policies to merge with
  parent Gateway/Listener policies rather than completely overriding them.

  Fixes envoyproxy#6734

Signed-off-by: Rajat Vig <rvig@etsy.com>
@zhaohuabing zhaohuabing force-pushed the feat/security-policy-merge branch from 95821a7 to c8b4bdc Compare January 13, 2026 01:23
Signed-off-by: Rajat Vig <rvig@etsy.com>
Signed-off-by: Rajat Vig <rvig@etsy.com>
@codecov
Copy link

codecov bot commented Jan 13, 2026

Codecov Report

❌ Patch coverage is 80.58824% with 33 lines in your changes missing coverage. Please review.
✅ Project coverage is 73.56%. Comparing base (38b0ad1) to head (215388d).

Files with missing lines Patch % Lines
internal/gatewayapi/securitypolicy.go 80.58% 29 Missing and 4 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #7918      +/-   ##
==========================================
+ Coverage   73.55%   73.56%   +0.01%     
==========================================
  Files         242      242              
  Lines       36949    37071     +122     
==========================================
+ Hits        27178    27272      +94     
- Misses       7851     7876      +25     
- Partials     1920     1923       +3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Signed-off-by: Rajat Vig <rvig@etsy.com>
Signed-off-by: Rajat Vig <rvig@etsy.com>
@arkodg arkodg requested a review from kkk777-7 January 14, 2026 05:03
Signed-off-by: Rajat Vig <rvig@etsy.com>
…y-merge

Signed-off-by: Rajat Vig <rvig@etsy.com>
Signed-off-by: Rajat Vig <rvig@etsy.com>
Signed-off-by: Rajat Vig <rvig@etsy.com>
@rajatvig rajatvig marked this pull request as ready for review January 18, 2026 23:30
@rajatvig rajatvig requested a review from a team as a code owner January 18, 2026 23:30
@rajatvig
Copy link
Contributor Author

@arkodg @kkk777-7 The PR is ready. Please do review when you have bandwidth. The E2E tests for the feature are passing, CI seems to have a bit of flakiness.

@arkodg arkodg added this to the v1.7.0-rc.1 Release milestone Jan 19, 2026
@kkk777-7
Copy link
Member

Hi @rajatvig , thanks for working on this!
I left some comments.

@kkk777-7
Copy link
Member

/retest

Signed-off-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com>
zhaohuabing
zhaohuabing previously approved these changes Jan 28, 2026
Copy link
Member

@zhaohuabing zhaohuabing left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

#7918 (comment) is tracked in #8088 and can be addressed later.

@kkk777-7
Copy link
Member

/retest

@kkk777-7
Copy link
Member

@rajatvig
Sorry, there’s one point I missed in the last review. I left a comment, so could you please take a look?

…t fix

Signed-off-by: Rajat Vig <rvig@etsy.com>
…y-merge

Signed-off-by: Rajat Vig <rvig@etsy.com>
@rajatvig
Copy link
Contributor Author

@rajatvig Sorry, there’s one point I missed in the last review. I left a comment, so could you please take a look?

Just fixed the last bit I think.

@rajatvig
Copy link
Contributor Author

rajatvig commented Feb 7, 2026

@kkk777-7 @zhaohuabing Does this look good now? Any other points I need to address?

@arkodg arkodg modified the milestones: Backlog, v1.8.0-rc.1 Release Feb 8, 2026
…y-merge

Signed-off-by: Rajat Vig <rvig@etsy.com>
…y-merge

Signed-off-by: Rajat Vig <rvig@etsy.com>
Signed-off-by: Rajat Vig <rvig@etsy.com>
Copy link
Member

@rudrakhp rudrakhp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that you might need to handle refs from parent policy after merge, see #8210 that fixes ref resolution for merged BTPs.

…y-merge

Signed-off-by: Rajat Vig <rvig@etsy.com>
@rajatvig
Copy link
Contributor Author

Note that you might need to handle refs from parent policy after merge, see #8210 that fixes ref resolution for merged BTPs.

@rudrakhp From what I could gather looking at the code, it would mostly affect secrets when the parent policy refers to a secret that the merged policy tries looking up in the route's namespace.

There is an issue #8094 tracking that. Would it be ok to resolve this as part of that issue?

@rudrakhp
Copy link
Member

@rajatvig it will affect not only secrets but any LocalObjectReference, I would recommend waiting on #8210 attempting to provide a generic way to handle local object references in merge scenarios.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Support MergeType in SecurityPolicy

5 participants